Skip to content

refactor(dialog): MD3 updates, updated Dialog API - #5072

Open
kachmashk wants to merge 18 commits into
callstack:mainfrom
kachmashk:refactor/dialog-improvements
Open

refactor(dialog): MD3 updates, updated Dialog API#5072
kachmashk wants to merge 18 commits into
callstack:mainfrom
kachmashk:refactor/dialog-improvements

Conversation

@kachmashk

@kachmashk kachmashk commented Aug 25, 2026

Copy link
Copy Markdown

Motivation

The goal of this PR is to address Material Design 3 guidelines for the Dialog component based on the official documentation as well as this issue. This PR also provides ability to avoid using previous compound components implementation in favor of props directly passed to the Dialog component.

Changes

Material Design 3:

  • Dialog.ScrollArea divider border color uses outline,
  • Dialog has now restricted minimum and maximum width set to 280dp and 560dp,
  • Fixed an issue when DialogIcon is the first component in the Dialog and it had a total top margin of 48dp instead of 24dp,
  • Dialog now detects if Dialog.Icon and Dialog.Title are present and if so enforcing Dialog.Title to be centered.

Dialog:

  • Dialog component now supports icon, title, content, actions, scrollable props instead of using compound components. Dialog still uses compound components underneath to properly comply with Material Design specs as these components enforce proper styling,
  • Added new test cases for Dialog to ensure the new approach works as intended.

Example:

  • Updated Dialog screen within Example app with the same dialogs as before but using new API to make sure they look and work as expected.

Related issue

Test plan

  • lint
  • typecheck
  • test

Reviewers can through new Dialogs added to the Example app. They are duplicated as previous dialogs but they use new approach.

Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/DialogScrollArea.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread docs/src/data/componentDocs6x.json
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx
Comment thread docs/src/data/componentDocs6x.json
Comment thread src/components/Dialog/DialogActions.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread src/components/Dialog/DialogTitle.tsx Outdated
Comment thread src/components/Modal.tsx
Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread docs/src/data/componentDocs6x.json
@kachmashk
kachmashk requested a review from JKobrynski September 1, 2026 08:40
@kachmashk
kachmashk force-pushed the refactor/dialog-improvements branch from 8562995 to 7be180f Compare September 2, 2026 08:36
@JKobrynski

JKobrynski commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

The MD3 corrections here should all land - divider colour, the 280/560 width clamp, the icon margin, the centred headline. What I'd like to reconsider is replacing children with title/content/actions props. #5076 solves the same problem while keeping the sub-components, which matches Card, List and DataTable. Could you drop the API change and keep the MD3 fixes? #5076 will need to land first either way, since the two touch the same files. @kachmashk

Comment thread src/components/Dialog/Dialog.tsx Outdated
Comment thread docs/6.x/docs/guides/migration.md
Comment thread docs/6.x/docs/guides/migration.md Outdated
Comment thread src/components/Modal.tsx
@JKobrynski

Copy link
Copy Markdown
Collaborator

Also @kachmashk could you resolve the comments that've been addressed? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants